-
Notifications
You must be signed in to change notification settings - Fork 351
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[2-quell-the-fracus] [Bug Fix] - Ensure that users hand typing tex into Numeric Inputs on Desktop do not cause an infinite loop. #2175
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We have a lot of old patterns in Perseus, and we would like to start to updating to more modern methods. This PR updates the Numeric Input logic in the following ways: 1. Moves all UI Related Numeric Input logic to a functional component, and creates a new `numeric-input.class.tsx` file for housing the static / class methods. 2. Removes all string refs related to Numeric Input in both the InputWithExamples, SimpleKeypadInput, and Tooltip components 3. Adds a little more specificity to method parameters Issue: LEMS-2443 - Manual Testing - Automated Tests - Landing onto a feature branch for future QA Regression pass Author: SonicScrewdriver Reviewers: SonicScrewdriver, mark-fitzgerald Required Reviewers: Approved By: mark-fitzgerald Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x) Pull Request URL: #2108
This PR is part of the Numeric Input Project, and will be landed onto the feature branch for a full QA pass. The intended goal was to remove all cases of underscore in the Numeric Input component, and to improve the commenting / documentation of the code. Some things to note: - I've changed the logic of `generateExamples` to match `shouldShowExamples`, as we were generating a list of all possible examples and simply not displaying it. This seemed unnecessary and we can exit both functions early. - I've added more specific types for `PerseusNumericInputWidgetOptions.simplify` Issue: LEMS-2446 - New tests - Manual testing in storybook Author: SonicScrewdriver Reviewers: SonicScrewdriver, mark-fitzgerald, jeremywiebe Required Reviewers: Approved By: mark-fitzgerald Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x) Pull Request URL: #2128
…er (#2121) ## Summary: This PR is part of the Numeric Input Project work. It is being landed onto the `feature/numeric-dx-refactor` branch. This PR contains the following changes: 1. Moves the InputWithExamples component to the NumericInput folder 2. Modernizes InputWithExamples to be a functional component 3. Addition of some comments Video Example of Snapshot Testing: https://github.com/user-attachments/assets/ca917778-50b0-46d2-89d8-dad95d1dadf2 Issue: LEMS-2785 ## Test plan: - Ensure all tests pass - Manual testing with PR Snapshot in upstream consumer - Landing onto feature branch that will see full QA regression pass before deployment Author: SonicScrewdriver Reviewers: SonicScrewdriver, mark-fitzgerald Required Reviewers: Approved By: mark-fitzgerald Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x) Pull Request URL: #2121
## Summary: This PR is part of the Numeric Input Project. The purpose of this PR is to improve our storybook setup for our Numeric Input Widget. This includes the following work: - Modernizing story structure - Hooking up argTypes with descriptions - Updating RendererWithDebugUI to also set customKeypad to the same value as isMobile - This allows us to ensure that our Widgets that use MathInput are properly updating when toggled into Mobile view - Adjustments to SideBySide - Automatically collapse the Perseus JSON view. - Moved the PerseusJSON view below the Renderer View - Rename to SplitView to better encapsulate the new design - Updated variable names to match [Current (Live) Storybook Example](https://khan.github.io/perseus/?path=/docs/perseus-widgets-numericinput--docs) | [PR Storybook Example](https://650db21c3f5d1b2f13c02952-osexoxinde.chromatic.com/?path=/docs/perseus-widgets-numeric-input--docs) Issue: LEMS-2449 ## Video Example: https://github.com/user-attachments/assets/69f6dbfb-1fda-445b-a06f-90a178f9dbeb ## Test plan: - Ensure all tests pass + manual testing Author: SonicScrewdriver Reviewers: SonicScrewdriver, mark-fitzgerald Required Reviewers: Approved By: mark-fitzgerald Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x) Pull Request URL: #2138
) ## Summary: This PR is part of the Numeric Input Project. This is a bug fix that ensures that the Examples Tooltip for the Numeric Input widget only displays examples from the _correct_ answers. Issue: LEMS-2803 ## Test plan: - Run tests - Creation of new snapshot test that verifies wrong answerFormats are not being provided. Author: SonicScrewdriver Reviewers: SonicScrewdriver, handeyeco, mark-fitzgerald Required Reviewers: Approved By: handeyeco, mark-fitzgerald Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x) Pull Request URL: #2159
…to Numeric Inputs on Desktop do not cause an infinite loop. I believe this solves the issue, but I'm putting this in a PR to perform some additional testing downstream. Issue: LEMS-198 Test Plan: - Run tests - New tex wrangler test
…reate infinite loop with incomplete tex in Numeric Input
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (ff226d9) and published it to npm. You Example: yarn add @khanacademy/perseus@PR2175 If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR2175 |
Size Change: +112 B (+0.01%) Total Size: 1.48 MB
ℹ️ View Unchanged
|
…reate infinite loop with incomplete tex in Numeric Input
182cc5e
to
985fb91
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
This ticket is part of the Numeric Input project.
Numeric Input has different experiences on Desktop versus Mobile:
This diverged experience resulted in the possibility to hit an infinite loop on Desktop if the user tries to hand type
\frac
or\dfrac
TeX commands , as the parser was unable to locate the next symbols to parse. This has likely been a bug since inception, but has become far more noticeable as we're now parsing answers on the fly to provide AI support. As a result, the answers are constantly being evaluated and would hit the infinite loop as soon as the user started typing the expressions.Issue: LEMS-198
Test plan: